-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add standalone executables #216
Conversation
|
|
requires gazebosim/gz-utils#2 |
src/ign/CMakeLists.txt
Outdated
@@ -0,0 +1,12 @@ | |||
add_executable( | |||
transport |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we currently embed the path of the ignition library in the cmdtransport.rb file:
- https://github.com/ignitionrobotics/ign-transport/blob/add_standalone/src/cmd/cmdtransport.rb.in#L31
- https://github.com/ignitionrobotics/ign-transport/blob/add_standalone/src/cmd/CMakeLists.txt#L10-L32
I'd like to do the same, but I think this cmake target needs to be in the same scope as the code that generates the .rb
file. Why not combine the cmd
and ign
folders? we could name it ign
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with that. I'll go ahead and do that as well as update the utils target
a60d1fd
to
647ee5a
Compare
the tests were not being compiled, so I re-enabled them in 30a14ef and fixed the configuration of the I noticed that calling for example:
|
I like splitting each verb into its own executable; I've updated the ruby script to call each one in ddd1da3 there are now just two test failures, but one of them is a seg-fault in the callback function passed to CLI: https://gist.github.com/scpeters/b18b1cbb38c43e0e02d406254aaeadef |
@osrf-jenkins run tests please |
opened osrf/homebrew-simulation#1320 to fix brew CI |
@osrf-jenkins run tests please |
brew build is failing because gazebosim/gz-cmake#143 has not yet been released |
@osrf-jenkins run tests please |
CMakeLists.txt
Outdated
@@ -74,6 +74,11 @@ else() | |||
ign_find_package(UUID REQUIRED) | |||
endif() | |||
|
|||
#-------------------------------------- | |||
# Find ignition-utils | |||
ign_find_package(ignition-utils1 REQUIRED COMPONENTS cli) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to say we could use PRIVATE
here because we aren't exporting the cli
component, but if this package starts using ImplPtr
or the suppression macros, then I'm not sure we'd be able to separate the core
and cli
components
Signed-off-by: Steve Peters <[email protected]>
I believe the abichecker failure is a false positive and have noted it in gazebo-tooling/release-tools#336 (comment) I think this is ready for review again, as I have addressed the problem noted by @caguero |
I believe this has been fixed |
Keep calling `ign topic --pub` if messages aren't received. Signed-off-by: Steve Peters <[email protected]>
I noticed some flaky failures of the |
flaky failure in GitHub action is unrelated to this PR and noted in #235 (comment) |
@caguero any other thoughts on this one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Example of using CLI11 to build standalone executables for use with the
ign
tools.This keeps the
ign.cc
andign.hh
the same, and just provides a C++ entrypoint.Signed-off-by: Michael Carroll [email protected]